Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for bulk devices on Windows and Mac #13008

Merged
merged 1 commit into from
May 13, 2024

Conversation

acolombier
Copy link
Member

@acolombier acolombier commented Mar 27, 2024

Allow bulk devices to be opened on Windows when using WinUSB and Mac.

@acolombier acolombier marked this pull request as ready for review March 27, 2024 18:24
Copy link
Member

@JoergAtGithub JoergAtGithub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some general suggestions. I've no USB Bulk device to test this.

src/controllers/bulk/bulkcontroller.cpp Outdated Show resolved Hide resolved
src/controllers/bulk/bulkcontroller.h Outdated Show resolved Hide resolved
src/controllers/controllermappinginfo.h Outdated Show resolved Hide resolved
@acolombier acolombier changed the title Support for bulk devices on Windows Support for bulk devices on Windows and Mac Mar 29, 2024
@acolombier
Copy link
Member Author

acolombier commented Mar 29, 2024

I took the initiative on expending the scope of this PR to also include Mac support as it seems it needs the same changes.

This was tested with S4 Mk3 screens

@JoergAtGithub
Copy link
Member

You need also to enabled BULK support in our Windows build setups:

CALL :AddCMakeVar2CMakeSettings_JSON "BULK" "BOOL" "False"

@acolombier acolombier force-pushed the fix/bulk-support-windows branch 3 times, most recently from 5447ce6 to 945e023 Compare March 29, 2024 11:08
@acolombier
Copy link
Member Author

Just a heads up here - there seems to be reports of issues on Zulip about Mac and that feature. Let's hold on merging till we have further details and potentially fix the usecase first.

I did some testing on MacOS M1 (Sonoma I believe) and it worked but there might be edge case other arch/OS version

@JoergAtGithub
Copy link
Member

I wonder what the kernel driver is, that needs to be detached, a generic one, that comes with the OS like WinUSB, or a Native Instruments driver for the S4 Mk3.
Maybe the detach of a Kernel driver is not necessary in case of Linux, just because this OS is not supported by Native Instruments?

@acolombier
Copy link
Member Author

Maybe the detach of a Kernel driver is not necessary in case of Linux, just because this OS is not supported by Native Instruments?

Yes, I think your theory is correct.
Do you think this means we should potentially rework the Linux part to make it compatible with the fact that NI may (very unlikely) release NI kernel drivers too (and so require a similar libusb opening procedure?

@JoergAtGithub
Copy link
Member

I'm not sure, but it would be good to test this without the NI driver package as well, to get a better understanding.
If the NI drivers are installed, we also need stop the service "NIHardwareService" on Windows, because it otherwise communicates via HID in parallel to Mixxx.

@acolombier
Copy link
Member Author

It's worth to mention I had to change the kernel driver from the NI to WinUSB one, so I guess this usecase isn't possible, or is it? I'm not sure how we could allow both kernels (And services) to be toggled on demand, and since I don't have much experience on Windows (nor the setup, and the interest to be honest), it's hard for me to work on a robust comprehensive solution here.

@acolombier
Copy link
Member Author

Could we consider merging this as-is as it is improving the overall bulk support on non-Linux platform (especially Mac, allowing bulk to work out of the box on MacOS 14).
Happy to create issues for the installer/further work to do on the Windows side to get that also working out of the box. In the meantime, I'd be happy to create a manual section to help installing WinUSB using some utility like Zadig

Copy link
Member

@JoergAtGithub JoergAtGithub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is fine now, and I will merge it therefore!
But this functionality is not enough, that a normal Windows user can use USB Bulk devices as on Linux. This is because the WinUSB driver registration for the device must be done manually by an Administrator using a tool like Zadig.

@JoergAtGithub JoergAtGithub merged commit 54a2083 into mixxxdj:main May 13, 2024
13 checks passed
@JoergAtGithub
Copy link
Member

Thank you, for addressing this long pending limitation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants